Skip to content

feat(server-utils): Migrate @opentelemetry/instrumentation-graphql to orchestrion#21885

Merged
chargome merged 10 commits into
developfrom
cg/orchestrion-graphql
Jul 9, 2026
Merged

feat(server-utils): Migrate @opentelemetry/instrumentation-graphql to orchestrion#21885
chargome merged 10 commits into
developfrom
cg/orchestrion-graphql

Conversation

@chargome

@chargome chargome commented Jul 1, 2026

Copy link
Copy Markdown
Member

Migrates @opentelemetry/instrumentation-graphql to orchestrion diagnostics-channel injection.

The channel based integration subscibes to orchestrion:graphql:{parse,validate,execute} and emits the same spans as the OTel integration — with a distinct auto.graphql.orchestrion.graphql origin.

closes #20752

@chargome chargome self-assigned this Jul 1, 2026
@chargome

chargome commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

bugbot run

Comment thread packages/node/src/sdk/experimentalUseDiagnosticsChannelInjection.ts Outdated
Comment thread packages/server-utils/src/integrations/tracing-channel/graphql/spans.ts Outdated
Comment thread packages/node/src/integrations/tracing/index.ts
@chargome

chargome commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

bugbot run

Comment thread packages/server-utils/src/integrations/tracing-channel/graphql/spans.ts Outdated
@chargome

chargome commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

bugbot run

Comment thread packages/server-utils/src/integrations/tracing-channel/graphql/constants.ts Outdated
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

⚠️ Warning: Base artifact is not the latest one, because the latest workflow run is not done yet. This may lead to incorrect results. Try to re-run all tests to get up to date results.

Path Size % Change Change
@sentry/browser 27.59 kB - -
@sentry/browser - with treeshaking flags 26.03 kB - -
@sentry/browser (incl. Tracing) 46.34 kB - -
@sentry/browser (incl. Tracing + Span Streaming) 48.13 kB - -
@sentry/browser (incl. Tracing, Profiling) 51.12 kB - -
@sentry/browser (incl. Tracing, Replay) 85.62 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 75.26 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 90.32 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 102.97 kB - -
@sentry/browser (incl. Feedback) 44.76 kB - -
@sentry/browser (incl. sendFeedback) 32.38 kB - -
@sentry/browser (incl. FeedbackAsync) 37.51 kB - -
@sentry/browser (incl. Metrics) 28.67 kB - -
@sentry/browser (incl. Logs) 28.91 kB - -
@sentry/browser (incl. Metrics & Logs) 29.59 kB - -
@sentry/react 29.38 kB - -
@sentry/react (incl. Tracing) 48.61 kB - -
@sentry/vue 33.03 kB - -
@sentry/vue (incl. Tracing) 48.24 kB - -
@sentry/svelte 27.61 kB - -
CDN Bundle 30 kB - -
CDN Bundle (incl. Tracing) 48.32 kB - -
CDN Bundle (incl. Logs, Metrics) 31.57 kB - -
CDN Bundle (incl. Tracing, Logs, Metrics) 49.64 kB - -
CDN Bundle (incl. Replay, Logs, Metrics) 70.81 kB - -
CDN Bundle (incl. Tracing, Replay) 85.84 kB - -
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) 87.14 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 91.64 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) 92.92 kB - -
CDN Bundle - uncompressed 89.35 kB - -
CDN Bundle (incl. Tracing) - uncompressed 146.1 kB - -
CDN Bundle (incl. Logs, Metrics) - uncompressed 94.05 kB - -
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed 150.07 kB - -
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed 218.78 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 265.3 kB - -
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed 269.26 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 279 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed 282.95 kB - -
@sentry/nextjs (client) 51.16 kB - -
@sentry/sveltekit (client) 46.75 kB - -
@sentry/core/server 78.42 kB - -
@sentry/core/browser 64.74 kB - -
@sentry/node-core 62.72 kB - -
@sentry/node 125.36 kB -0.01% -1 B 🔽
@sentry/node (incl. diagnostics channel injection) 136.41 kB +1.05% +1.41 kB 🔺
@sentry/node/import (ESM hook with diagnostics-channel injection) 69.95 kB - -
@sentry/node/light 50.72 kB - -
@sentry/node - without tracing 74.05 kB - -
@sentry/aws-serverless 85.5 kB - -
@sentry/cloudflare (withSentry) - minified 181.71 kB - -
@sentry/cloudflare (withSentry) 449.16 kB - -

View base workflow run

@chargome

chargome commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

bugbot run

@chargome

chargome commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 43d4510. Configure here.

Comment thread packages/node/src/sdk/experimentalUseDiagnosticsChannelInjection.ts Outdated
@chargome chargome force-pushed the cg/orchestrion-graphql branch from 0eccdf9 to d2c7ce0 Compare July 6, 2026 08:25
…to orchestrion

Rewrite the graphql integration as a diagnostics-channel subscriber, with orchestrion
injecting channels into graphql's compiled `parse`/`validate`/`execute` (v14–v16). Registers
`graphqlIntegration` in the channelIntegrations registry and adds a per-lib config file, so the
opt-in and `diagnosticsChannelInjectionIntegrations()` pick it up automatically. Span
names/attributes/origin mirror the OTel integration (distinct `auto.graphql.orchestrion.graphql`
origin); the structural graphql types are shared with the vendored OTel instrumentation.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chargome chargome force-pushed the cg/orchestrion-graphql branch from d2c7ce0 to 21acce6 Compare July 7, 2026 09:11
@chargome chargome marked this pull request as ready for review July 8, 2026 09:09
@chargome chargome requested a review from a team as a code owner July 8, 2026 09:09
@chargome chargome requested review from JPeer264, andreiborza, isaacs, mydea and nicohrubec and removed request for a team July 8, 2026 09:09
@chargome chargome marked this pull request as draft July 8, 2026 09:32
chargome and others added 3 commits July 8, 2026 11:42
…phql

# Conflicts:
#	packages/server-utils/src/orchestrion/index.ts
…er conventions

develop landed a native diagnostics_channel graphql subscriber (v17) with new span conventions
(graphql.document, WEB_SERVER_GRAPHQL_SPAN_OP op, auto.graphql.diagnostic_channel origin). Rework the
orchestrion v14–16 subscriber to reuse that module's utils/conventions so spans are identical across
versions, and compose the registry entry as native(v17) + orchestrion(v14–16) via extendIntegration so
opting into injection instruments every supported version without the OTel patcher (no v17 regression).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…phql

# Conflicts:
#	packages/server-utils/src/orchestrion/channels.ts
#	packages/server-utils/src/orchestrion/config/index.ts
@chargome chargome marked this pull request as ready for review July 8, 2026 11:33
@chargome chargome requested a review from logaretm July 8, 2026 12:24
chargome added 2 commits July 8, 2026 14:46
…phql

# Conflicts:
#	packages/server-utils/src/orchestrion/channels.ts
#	packages/server-utils/src/orchestrion/config/index.ts
#	packages/server-utils/src/orchestrion/index.ts
Comment thread packages/server-utils/test/graphql.test.ts Outdated
Comment thread packages/server-utils/src/orchestrion/index.ts Outdated

@andreiborza andreiborza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for taking care of this!

andreiborza added a commit that referenced this pull request Jul 8, 2026
…c-context logic (#22099)

Removes the ioredis unit test that stubbed the SDK's async context
strategy to drive spans, which replicated SDK internals in the test
layer. Per the review thread in #21885, we rely on the integration tests
instead.

The orchestrion path is covered by the `redis` (ioredis scenario) and
`redis-cache` node integration suites. The `responseHook` and
connect-span cases aren't cleanly testable on the orchestrion path (the
hook is wired internally for caching) and these suites are Docker-gated,
so they're left as a possible follow-up rather than migrated now.

Ref:
#21885 (comment)

Co-authored-by: Opus 4.8 <noreply@anthropic.com>
andreiborza added a commit that referenced this pull request Jul 8, 2026
…nc-context logic (#22101)

Removes the mongoose unit test that stubbed the SDK's async context
strategy to drive spans, which replicated SDK internals in the test
layer. Per the review thread in #21885, we rely on the integration tests
instead.

The path is covered by the `mongoose-tracing-channel` node integration
suite. To avoid losing the useful unit-only assertions, this PR also
adds error-status, empty-filter, and cursor-batchSize coverage to that
suite.

Ref:
#21885 (comment)

---------

Co-authored-by: Opus 4.8 <noreply@anthropic.com>
andreiborza added a commit that referenced this pull request Jul 8, 2026
…-context logic (#22098)

Follow-up to the review discussion in #21885: several `server-utils`
diagnostics-channel unit tests stub the SDK's async context strategy
(`setAsyncContextStrategy` plus a hand-rolled
`installTestAsyncContextStrategy`) to drive spans, replicating SDK
internals in the test layer. Per the thread, we're removing these in
favor of the integration tests that exercise the real code paths.

This PR removes both redis unit tests:
- `test/integrations/tracing-channel/redis.test.ts` (orchestrion path,
origin `auto.db.orchestrion.redis`) — covered by `suites/tracing/redis`
(the suite branches on `isOrchestrionEnabled()`).
- `test/redis/redis-dc-subscriber.test.ts` (native diagnostics-channel
subscriber, origin `auto.db.redis.diagnostic_channel`) — covered by
`suites/tracing/redis-dc`.

Ref:
#21885 (comment)

Co-authored-by: Opus 4.8 <noreply@anthropic.com>
andreiborza added a commit that referenced this pull request Jul 8, 2026
…async-context logic (#22097)

Removes the postgres.js unit test that stubbed the SDK's async context
strategy to drive spans, which replicated SDK internals in the test
layer. Per the review thread in #21885, we rely on the integration tests
instead.

The orchestrion path is already covered end to end by the `postgresjs`
node integration suite. The remaining unit-only assertions checked
internal implementation details (not re-creating a span on a re-entrant
call, skipping an already-wrapped query, no span without an active
parent) rather than the spans a user actually sees, so there was nothing
meaningful to move into an integration test.

Ref:
#21885 (comment)

Co-authored-by: Opus 4.8 <noreply@anthropic.com>
andreiborza added a commit that referenced this pull request Jul 8, 2026
…nc-context logic (#22103)

Removes the three graphql subscriber unit tests and their shared helper,
which stubbed the SDK's async context strategy and a fake document to
drive spans, replicating SDK internals in the test layer. Per the review
thread in #21885, we rely on the integration tests instead.

The path is covered by the `graphql-tracing-channel` node integration
suite. To avoid losing the useful unit-only assertions, this PR also
adds validation-error status and `ignoreTrivialResolveSpans` coverage to
that suite. The subscription-channel case is left as a possible
follow-up.

Ref:
#21885 (comment)

---------

Co-authored-by: Opus 4.8 <noreply@anthropic.com>
andreiborza added a commit that referenced this pull request Jul 8, 2026
…ontext logic (#22100)

Removes the hapi unit test that stubbed the SDK's async context strategy
to drive spans, which replicated SDK internals in the test layer. Per
the review thread in #21885, we rely on the integration tests instead.

The orchestrion path is covered by the `hapi` node integration suite.
`hapi-utils.test.ts` stays (pure metadata formatting, no async-context
stubbing). The remaining unit-only assertions checked internal
implementation details (setup only wrapping once when called twice, no
span without an active span, accepting the ext handler as a tuple or an
object) rather than the spans a user actually sees, so there was nothing
meaningful to move into an integration test.

Ref:
#21885 (comment)

Co-authored-by: Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rewrite @opentelemetry/instrumentation-graphql to orchestrion

3 participants